-
Notifications
You must be signed in to change notification settings - Fork 135
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
improvement: fontawesome, next part of icons #1418
Conversation
Puścisz ikonki? |
Jest kolizja, bo równolegle przeniosłem wszystko na: {button ...} Obeszło się bez robienia oddzielnych elementów button i link. Po prostu w button dałem dodatkowy type o wartości link. |
To nie było równolegle :P Mój PR był wcześniej. git merge tak wygląda czasami :D |
Spróbuj w swoich zmianach od razu przyciski (tam gdzie używasz) przenieść na nową funkcję smarty {button}. |
Przejrzałem te ostatnie zmiany - kierunek raczej będzie nie ten, tzn. nie wiem czy sensowne jest pakowanie wszystkie w klasy lms-ui-icon-content tylko po to, żeby uzyskać odstęp. Raczej takie coś wypadałoby wbudować w bloki {box_...} i {tab_...}. |
Jeśli coś jest bardzo nagminne nie warto szpikować dodatkowym kodem html. |
Dlatego też proponuję na razie z ikonkami nie ruszać szablonów, które są zrobione na tabelkach, bo lepiej wypracować jeden/kilka mechanizmów na mniejszej liczbie tabel niż potem wszystko w kółko modyfikować. |
W związku z tym proponuję ograniczyć się ze zmianami ikon tylko do przeniesionych na nowy layout szablonów. Mam na myśli te ikonki przy każdym polu. |
Jakoś specjalnie buttonów nie tykałem bo wiedziałem że kod się zmieni. Przejrzę je wszystkie ale dopiero pojutrze. Zrób merge&squash, żeby znów się to nie rozjechało. |
Klasę lms-ui-content łatwo wykasować. Gdy przeniesiemy definicję ikon na nowych kod {icon} to szybciej zmienimy kod bo dodałem sporo klas do style.less. I łatwiej będzie to z automatu zmienić. |
Wątpię - zrobi się tylko większy bałagan. |
Co tam powinno być zamiast 6134? :) |
Poprawiłem to. Nie wyłapałem w webeditorze przy przeglądaniu PR, dzięki. |
Część ikon jest w <IMG SRC dużo więcej w <i class. Chcę doprowadzić do sytuacji gdzie będą wszystkie z fontawesome. |
Widziałem, że przy okazji wiele rzeczy zmieniasz, a nie tędy droga - bałagan murowany. |
Zauważyłem, że przy okazji przestało w którymś momencie działać tworzenie zdarzeń terminarza od razu powiązanych ze zgłoszeniem :( |
Sugeruję nie spieszyć się ze zmianami, bo to potem na jakość odbija się mocno. |
Zrobiłem test - https://demo.lms.org.pl/?m=eventinfo&id=83. Działa |
Ale fakt, że jeśli w _GET jest ticketid określony checkbox nie powinien się wyświetlać. |
Ogólna uwaga - skoro już robimy powtarzające się elementy zbudowane z kilku horyzontalnie położonych buttonów to też już to mamy (albo prawie mamy) w smarty. |
Nie. W tym PR tylko ikonki. Jedynie kilka ikon merytorycznie podmieniłem po konsultacji na inne. |
A całkowite reformatowania pliku? :) |
Co prawda PHP Storm daje CTRL+ALF+F lub CTRL+ALT+SHIFT+F. |
A zmiany linków na przyciski? |
Może dla przykładu zróbmy jeden szablon wspólnie od początku do końca i zobaczymy czy jeszcze w ogóle czegoś brakuje ze strony smarty? |
Btw. Commitem https://github.com/chilek/lms-plus/commit/70be37dda22b315fc1ac8d887e05bddb0524ad1a |
Wydaje mi się, że niewiele brakuje od strony smarty, żeby móc zmieniać szablony html, na szablony używające elementów smarty. Są zaimplementowane: |
Dla zakładek {box_...} fajnie byłoby pójść za ciosem i wszystkie przerobić w CORE. Tym bardziej, że procent zaawansowania tego przedsięwzięcia jest wysoki - zaadaptowane są już prawie wszystkie zakładki klienta i komputera. |
https://demo.lms.org.pl/?m=eventedit&id=84 - zazanaczona domyślnie opcja "przypisz do zgłoszenia" ale wartość jej pustaw - tak być nie powinno ponieważ zdarzenie nie jest nigdzie przypisane. |
@chilek chciałem dokończyć ten PR, Było: Proponowałem: Jak według Ciebie powinien być wprowadzony ten kod z obrazkami daj proszę przykład na przykładzie tego fragmentu. |
Mi się jedynie nie podoba: lms-ui-icon-content, bo wszędzie trzeba to wstawiać, a po co? |
Jak zrobić inaczej odstępy? |
Chodzi o odstępy między ikonkami i etykietami tekstowymi? Moim zdaniem całość powinna trafiać do jakiegoś elementu html, który to opakowuje. |
Tak. |
W sumie niektóre elementy są już opakowane w nadrzędny element TD. Można byłoby dla takiego elementu wprowadzić tymczasowo jakąś klasę CSS. Kiedyś pewnie TD wylecą, ale kiedy - daleka przyszłość pewnie. |
Ok - usunąłem lms-ui-icon-content z kodu. Pokaż proszę przykład jak opisać elementy TD w CSS tam gdzie znajdują się ikonki tak, żeby były wyśrodkowane względem siebie. Mi się coś rozjeżdza. |
<TD class="lms-ui-normal-column">
<i ...></i><span class="bold">Label</span>
</TD> .lmsbox .lms-ui-normal-column > :not(:last-child) {
margin-right: 0.2em;
} Nie testowałem tego powyżej, ale idea została przestawiona. Może warto byłoby jakoś uogólnić to dla table lmsbox* tak, żeby od razu w tabeli td dowolny się łapał na te definicje css? |
A jak z wyśrodkowaniem w pionie? |
Tzn. co konkretnie chcesz uzyskać? |
Wyśrodkowanie w pionie ikonki zawartej w znaczniuk |
.lmsbox .lms-ui-normal-column {
vertical-align: middle;
} Utwórz może w jsfiddle przykład i mi udostępnić - tam ustalimy co i jak. |
Przykładem może być plik rtticketinfobox.html |
Widzisz moje zmiany na jsfiddle? |
Taki podałeś przykład trochę nie z życia - ikonki są różnych szerokości a jednak przydałoby się żeby:
Jeśli nie widzisz moich zmian to podmieniłem kod na:
|
Jaki masz tam login? |
Prawdopodobnie nie dołączyłeś do współpracy w moim jsfiddle, bo nie mam Ciebie na liście współpracowników. |
Coś takiego? |
Tak. Tak jest git. Spróbuję to jutro wrzucić w kilka klas i potestować. |
Wiesz co? Próbowałem wrzucić to do schemat z jsfiddle.net do kodu rtticketinfobox.html (011a208). Czy ten span z boldem jest na prawdę potrzebny? |
Oj wydaje mi się, że to powinno być inaczej zrobione. |
To tylko przykład, ale jak byś chciał robić bardziej docelowo to spójrz na parę szablonów zrobionych w oparciu o div:flexbox (oparte o funkcje i bloki smarty). |
Ten PR pozwolił wypracować sposób kodowania przycisków i ikon. Zamykam bo jest za wiele konfliktów zmiany trzeba wprowadzić jeszcze raz. |
No description provided.